Skip to content

Fix typescript constructor parameter properties#262

Closed
soh335 wants to merge 1 commit into
react:masterfrom
soh335:fix/typescript-constructor-parameter-properties
Closed

Fix typescript constructor parameter properties#262
soh335 wants to merge 1 commit into
react:masterfrom
soh335:fix/typescript-constructor-parameter-properties

Conversation

@soh335

@soh335 soh335 commented Sep 21, 2018

Copy link
Copy Markdown
Contributor

Summary

issue #258.
metro-react-native-babel-preset don't recognize typescript constructor parameter properties, currently.

Test plan

not yet.

plugin-transform-classes drop typescript constructor parameter
propetries syntax.
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@soh335 soh335 changed the title Load plugin-transfrom-typescript before plugin-transform-classes. Fix typescript constructor parameter properties Sep 21, 2018
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #262 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #262   +/-   ##
=======================================
  Coverage   85.21%   85.21%           
=======================================
  Files         141      141           
  Lines        4533     4533           
  Branches      706      706           
=======================================
  Hits         3863     3863           
  Misses        598      598           
  Partials       72       72
Impacted Files Coverage Δ
...etro-react-native-babel-preset/src/configs/main.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b4f97f...8621965. Read the comment docs.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 21, 2018
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@rafeca rafeca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR! It looks great 😍

Can you do the small improvement to the test? After that it's ready to land 😃

@soh335

soh335 commented Sep 21, 2018

Copy link
Copy Markdown
Contributor Author

@rafeca I don't have plan to commit. But it seems not to be tested about constructor parameter properties. I can't find test codes of after transpiling typescripts. Is it ok ?

@rafeca

rafeca commented Sep 24, 2018

Copy link
Copy Markdown
Contributor

Yes, this is fine, thanks a lot for the PR! I'm going to land it 😄

@facebook-github-bot facebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rafeca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@soh335

soh335 commented Oct 1, 2018

Copy link
Copy Markdown
Contributor Author

@rafeca any progress ?

aleclarson pushed a commit to aleclarson/metro that referenced this pull request May 27, 2019
Summary:
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->

**Summary**

<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->

issue react#258.
metro-react-native-babel-preset don't recognize typescript constructor parameter properties, currently.

**Test plan**

<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->

not yet.
Pull Request resolved: react#262

Reviewed By: mjesun

Differential Revision: D10009176

Pulled By: rafeca

fbshipit-source-id: 62f6279658464df127033aa322c76f77c9694aed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants